Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make anyone can register block syncer code to the Glueby::BlockSyncer #66

Merged
merged 10 commits into from Jun 30, 2021
Merged

Make anyone can register block syncer code to the Glueby::BlockSyncer #66

merged 10 commits into from Jun 30, 2021

Conversation

rantan
Copy link
Contributor

@rantan rantan commented Jun 14, 2021 •
edited

See the example code in class document here

@rantan rantan requested a review from azuchi June 14, 2021 06:09
@rantan rantan marked this pull request as ready for review June 14, 2021 06:09
azuchi
azuchi requested changes Jun 14, 2021
View reviewed changes
Copy link
Contributor

@azuchi azuchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to this change, following change are required:

  • remove wallet_adaptor.rake. This is same with ActiveRecordWalletAdapter::Syncer, right?
  • need to migrate timestam.rake.

lib/glueby/block_syncer.rb Outdated Show resolved Hide resolved
spec/tasks/glueby/contract/block_syncer_spec.rb Outdated Show resolved Hide resolved
@rantan
Copy link
Contributor Author

rantan commented Jun 14, 2021

remove wallet_adaptor.rake. This is same with ActiveRecordWalletAdapter::Syncer, right?

It is almost the same but ActiveRecordWalletAdapter::Syncer is not a rake task.
How about modifying that the wallet_adapter.rake uses ActiveRecordWalletAdapter::Syncer internally.
I wonder if the wallet_adapter rake task has use cases. I actually don't know. If not, removing wallet_adapter.rake is good for me.

@rantan
Copy link
Contributor Author

rantan commented Jun 14, 2021

@azuchi

need to migrate timestam.rake.

You mean glueby:contract:timestamp:confirm should be abrogated and the confirmation should be checked as a block syncer logic.

@rantan
Copy link
Contributor Author

rantan commented Jun 14, 2021 •
edited

  • I will remove wallet_adapter.rake
  • the timestamp confirmation should be checked as a block syncer logic.
  • Add README.md documentation how to use BlockSyncer in applications
  • Update initilalizer generator to the latest specs.

@rantan rantan force-pushed the modify_block_syncer branch 3 times, most recently from f6834bd to 6d64df9 Compare June 29, 2021 00:43
@rantan
Copy link
Contributor Author

rantan commented Jun 29, 2021

I've updated all the above things.

@rantan rantan requested a review from azuchi June 29, 2021 00:56
azuchi
azuchi reviewed Jun 30, 2021
View reviewed changes
lib/glueby.rb Outdated
@@ -1,6 +1,12 @@
require "glueby/version"
require 'tapyrus'

begin
require 'rails'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why require rails? It doesn't seem to be defined in the gemspec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that it is necessary if glueby is loaded in a rails project. Glueby needs to use Rails::Railtie module. However, I didn't confirm whether it causes some error or not. I will check it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed these codes. There is no error if it is removed.

rantan added 10 commits June 30, 2021 16:55
@rantan
Move Glueby::Contract::Task::BlockSyncer to Glueby::BlockSyncer
23939bb 
To be referred from other module and etc.
@rantan
Make BlockSyncer accept any syncer logic by registration
15b9bef
@rantan
Move block_syncer.rake out from contract namespace
5f65ca7
@rantan
Remove wallet_adapter.rake
2cb18b9 
We can use glueby:block_syncer:start
@rantan
Add BlockSyncer usage document
a89e31d
@rantan
Reconstruct README.md document
d5dd0b7
@rantan
Update initializer template to use Glueby.configure
36309c0
@rantan
Make syncing timestamp confirmation use BlockSyncer
b62ede5
@rantan
Fix timestamp tx synchronous issue in use of FeeProvider
a1c36b5 
Glueby::Internal::Wallet#broadcast(tx) changes the txid of tx in arguments while using FeeProvider. If it store a txid before broadcasting, the txid would be changed.
Here the method can receive a code block which is called before actual broadcasting and caller can execute some codes with real tx that is going to be broadcasted.
@rantan
Remove unnecessary require rails
d564762
@azuchi azuchi merged commit d707138 into chaintope:master Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azuchi azuchi azuchi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rantan @azuchi